Skip to content

fix: update rbac to support modern event recorder events.k8s.io - #52

Merged
stubbi merged 2 commits into
paperclipinc:mainfrom
kbntx-org:update_rbac_to_support_modern_api_recorder
Aug 25, 2026
Merged

fix: update rbac to support modern event recorder events.k8s.io#52
stubbi merged 2 commits into
paperclipinc:mainfrom
kbntx-org:update_rbac_to_support_modern_api_recorder

Conversation

@kbntx

@kbntx kbntx commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

The Helm chart RBAC was only granting event write access to the core API group (""). However in the codebase the controller-runtime c.recorder = m.GetEventRecorder(c.Name()) is calling the new api group under events.k8s.io. This lead to the controller emitting errors like the below one:

"error":"events.events.k8s.io is forbidden: User \"system:serviceaccount:karpenter:karpenter\" cannot create resource \"events\" in API group \"events.k8s.io\" in the namespace \"default\""

I kept the group "" in the RBAC because the karpenter package upstream is still using the old legacy events api in some places like mgr.GetEventRecorderFor("karpenter") in https://github.com/kubernetes-sigs/karpenter/blob/2266468104f36373ed002241afaee2d3bbc26078/pkg/controllers/controllers.go.

Changes

  • Updated the rbac.yaml to allow to emit event with the new recorder API

Verification

  • make test passes
  • make lint passes
  • make generate-verify passes (CRD/deepcopy up to date)
  • Added/updated tests for the change

@kbntx

kbntx commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Hey @stubbi, this is my first contribution to a public repo, I found this little issue when using the provider on my cluster. (thanks a lot for open sourcing it!)

Let me know if anything is not following the guidelines or if a change is required.

@stubbi stubbi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified against the code: pkg/controllers/nodeclass/controller.go:193 uses m.GetEventRecorder(), which returns a k8s.io/client-go/tools/events recorder writing events.k8s.io Events — the chart only granted the core "" group, so the forbidden error is real. Keeping "" for karpenter core's legacy recorder is right, and create+patch on both groups matches what the new recorder needs for event series. CI is green apart from govulncheck, which fails on stdlib vulns fixed on main by #57 — a rerun after that merges clears it. Chart version bump can ride the next release chore.

@kbntx

kbntx commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Verified against the code: pkg/controllers/nodeclass/controller.go:193 uses m.GetEventRecorder(), which returns a k8s.io/client-go/tools/events recorder writing events.k8s.io Events — the chart only granted the core "" group, so the forbidden error is real. Keeping "" for karpenter core's legacy recorder is right, and create+patch on both groups matches what the new recorder needs for event series. CI is green apart from govulncheck, which fails on stdlib vulns fixed on main by #57 — a rerun after that merges clears it. Chart version bump can ride the next release chore.

Awesome, I updated the branch to get #57 but I guess it was not really needed, just need to re-run the CI. Thank you!

@stubbi
stubbi merged commit 3420a64 into paperclipinc:main Aug 25, 2026
6 checks passed
@stubbi stubbi mentioned this pull request Aug 26, 2026
stubbi added a commit that referenced this pull request Aug 26, 2026
Chart 2.1.1 / appVersion 2.1.1. Ships the events.k8s.io RBAC grant
(#52), the Go 1.26.7 toolchain rebuild clearing five stdlib CVEs (#57),
and dependency bumps (karpenter 1.14.1, hcloud-go 2.47.0, k8s.io
0.36.4) (#58).


Claude-Session: https://claude.ai/code/session_01FJnTGiMuTJSFUVRRqjfTbV

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants